Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Iox #615 unblock app shutdown with blocked publisher #708

Merged
merged 7 commits into from
Apr 12, 2021
Merged

Iox #615 unblock app shutdown with blocked publisher #708

merged 7 commits into from
Apr 12, 2021

Conversation

elBoberido
Copy link
Member

@elBoberido elBoberido commented Apr 12, 2021 •
edited
Loading

Pre-Review Checklist for the PR Author

  1. Code follows the coding style of CONTRIBUTING.md
  2. Tests follow the best practice for testing
  3. Branch follows the naming format (iox-#123-this-is-a-branch)
  4. Commits messages are according to this guideline
    • Commit messages have the issue ID (iox-#123 commit text)
    • Commit messages are signed (git commit -s)
    • Commit author matches Eclipse Contributor Agreement (and ECA is signed)
  5. Update the PR title
    • Follow the same conventions as for commit messages
    • Link to the relevant issue
  6. Relevant issues are linked
  7. Add sensible notes for the reviewer
  8. All checks have passed (except task-list-completed)
  9. Assign PR to reviewer

Notes for Reviewer

Same as #697 but this time for master

Checklist for the PR Reviewer

  • Commits are properly organized and messages are according to the guideline
  • Code according to our coding style and naming conventions
  • Unit tests have been written for new behavior
  • Public API changes are documented via doxygen
  • Copyright owner are updated in the changed files
  • PR title describes the changes

Post-review Checklist for the PR Author

  1. All open points are addressed and tracked via issues

References

@elBoberido
iox-eclipse-iceoryx#615 make ProcessManager and PortManager handle a …
c348598 
…process shutdown preparation request

Signed-off-by: Mathias Kraus <mathias.kraus@apex.ai>
@elBoberido
iox-eclipse-iceoryx#615 handle PREPARE_APP_TERMINATION in RouDi
a1e4c4f 
Signed-off-by: Mathias Kraus <mathias.kraus@apex.ai>
@elBoberido
iox-eclipse-iceoryx#615 add PoshRuntime::shutdown to unblock app shut…
0b2b481 
…down there are blocking publisher

Signed-off-by: Mathias Kraus <mathias.kraus@apex.ai>
@elBoberido
iox-eclipse-iceoryx#615 add test for PoshRuntime to check if shutdown…
db4be23 
… unblocks a blocked publisher

Signed-off-by: Mathias Kraus <mathias.kraus@apex.ai>
@elBoberido
iox-eclipse-iceoryx#615 fix documentation
9a347c9 
Signed-off-by: Mathias Kraus <mathias.kraus@apex.ai>
@elBoberido
iox-eclipse-iceoryx#615 update example readme
eb9c97b 
Signed-off-by: Mathias Kraus <mathias.kraus@apex.ai>
@elBoberido
iox-eclipse-iceoryx#615 extend C API with the runtime shutdown method
4bb9cbe 
Signed-off-by: Mathias Kraus <mathias.kraus@apex.ai>
@elBoberido elBoberido self-assigned this Apr 12, 2021
@elBoberido elBoberido added the bugfix Solves a bug label Apr 12, 2021
@codecov
Copy link

codecov bot commented Apr 12, 2021 •
edited
Loading

Codecov Report

Merging #708 (4bb9cbe) into master (94d0ea6) will increase coverage by 0.01%.
The diff coverage is 63.63%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #708      +/-   ##
==========================================
+ Coverage   74.50%   74.52%   +0.01%     
==========================================
  Files         315      315              
  Lines       11213    11253      +40     
  Branches     1969     1975       +6     
==========================================
+ Hits         8354     8386      +32     
- Misses       2083     2088       +5     
- Partials      776      779       +3
Flag Coverage Δ
unittests 74.47% <63.63%> (+0.01%) ⬆️
unittests_timing 31.21% <2.27%> (-0.11%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
iceoryx_binding_c/source/c_runtime.cpp 52.94% <0.00%> (-7.06%) ⬇️
...clude/iceoryx_posh/internal/roudi/port_manager.hpp 100.00% <ø> (ø)
...de/iceoryx_posh/internal/roudi/process_manager.hpp 100.00% <ø> (ø)
...eoryx_posh/internal/runtime/ipc_interface_base.hpp 100.00% <ø> (ø)
iceoryx_posh/source/roudi/roudi.cpp 55.88% <50.00%> (-0.22%) ⬇️
iceoryx_posh/source/runtime/posh_runtime.cpp 65.95% <52.63%> (-0.72%) ⬇️
iceoryx_posh/source/roudi/process_manager.cpp 64.79% <80.00%> (+0.65%) ⬆️
iceoryx_posh/source/roudi/port_manager.cpp 78.20% <100.00%> (+0.42%) ⬆️
...ils/include/iceoryx_utils/internal/cxx/smart_c.inl 89.06% <0.00%> (ø)
iceoryx_utils/source/posix_wrapper/timer.cpp 60.96% <0.00%> (+0.43%) ⬆️
... and 2 more

elfenpiff
elfenpiff reviewed Apr 12, 2021
View reviewed changes
iceoryx_posh/source/runtime/posh_runtime.cpp
IpcMessage receiveBuffer;

if (m_ipcChannelInterface.sendRequestToRouDi(sendBuffer, receiveBuffer)
&& (1U == receiveBuffer.getNumberOfElements()))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uuuhh Yoda notation you use.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

c&p I used :)

iceoryx_posh/test/moduletests/test_roudi_process_manager.cpp
ASSERT_TRUE(publisher.isOffered());

m_sut->handleProcessShutdownPreparationRequest(m_processname);

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens when I call here publisher.offer() again? Could we test this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well, in this case the app might be again blocked. It's the responsibility of the application developer to not do this since we have no/less chance to prevent this

@elBoberido elBoberido merged commit 473090d into eclipse-iceoryx:master Apr 12, 2021
@elBoberido elBoberido deleted the iox-#615-unblock-app-shutdown-with-blocked-publisher branch April 12, 2021 14:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@elfenpiff elfenpiff elfenpiff left review comments

@budrus budrus budrus approved these changes

@FerdinandSpitzschnueffler FerdinandSpitzschnueffler FerdinandSpitzschnueffler approved these changes

@mossmaurice mossmaurice Awaiting requested review from mossmaurice

Assignees

@elBoberido elBoberido

Labels
bugfix Solves a bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Possibility to block the publisher when subscriber queue is full
4 participants
@elBoberido @budrus @elfenpiff @FerdinandSpitzschnueffler